Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expose ModelsService.changes and listen to that in ChatController #5578

Merged
merged 1 commit into from
Sep 15, 2024

Conversation

sqs
Copy link
Member

@sqs sqs commented Sep 14, 2024

Previously, the ChatController listened for auth changes and assumed that model changes would have been already applied. This required a specific ordering of operations in the AuthProvider, which was a brittle way of ensuring this behavior (and the chat model selector e2e test broke when that behavior was slightly changed). Now, the ModelsService exposes a changes observable and the ChatController listens to that to follow changes to models, which makes sense.

Test plan

e2e tests cover this well.

@sqs sqs force-pushed the sqs/modelssvc-observe branch 2 times, most recently from 768a625 to 3523799 Compare September 15, 2024 05:55
@sqs sqs changed the title make ModelsService observe config and auth expose ModelsService.changes and listen to that in ChatController Sep 15, 2024
@sqs sqs marked this pull request as ready for review September 15, 2024 05:57
@sqs sqs requested a review from a team September 15, 2024 05:57
Previously, the ChatController listened for auth changes and assumed that model changes would have been already applied. This required a specific ordering of operations in the AuthProvider, which was a brittle way of ensuring this behavior (and the `chat model selector` e2e test broke when that behavior was slightly changed). Now, the ModelsService exposes a `changes` observable and the ChatController listens to that to follow changes to models, which makes sense.
@sqs sqs enabled auto-merge (squash) September 15, 2024 06:40
@sqs sqs merged commit c758d5a into main Sep 15, 2024
18 of 19 checks passed
@sqs sqs deleted the sqs/modelssvc-observe branch September 15, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants